feat(TaskRunSpec) : Add SchedulerName to TaskRunSpec #1790
Conversation
googlebot
added
the
cla: yes
tekton-robot
added
size/XS
|
Hi @waveywaves. Thanks for your PR. I'm waiting for a tektoncd member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
|
/ok-to-test Please add a test in pod_test.go to ensure this behaves as you expect. |
tekton-robot
added
ok-to-test
waveywaves
changed the title
tekton-robot
added
size/M
waveywaves
force-pushed
the
feat/scheduler-name
branch
from
dd018a8 to
66d7fef
Compare
|
@imjasonh I have added the tests, should I add the documentation as well ? |
waveywaves
changed the title
@waveywaves yes pleas 👼 🤗 |
|
|
||
| // SchedulerName specifies the scheduler to be used to dispath the Pod | ||
| // +optional | ||
| SchedulerName string `json:"schedulerName"` |
There was a problem hiding this comment.
I think we should put this in PodTemplate, that's where we've been putting fields that are specific to how the underlying pod will be configured.
There was a problem hiding this comment.
I have updated the PR accordingly.
waveywaves
force-pushed
the
feat/scheduler-name
branch
2 times, most recently
from
36c2d7b to
c842b3f
Compare
|
/retest |
pkg/apis/pipeline/v1alpha1/pod.go
Outdated
| @@ -87,4 +87,8 @@ type PodTemplate struct { | |||
| // default. | |||
| // +optional | |||
| PriorityClassName *string `json:"priorityClassName,omitempty" protobuf:"bytes,7,opt,name=priorityClassName"` | |||
|
|
|||
| // SchedulerName specifies the scheduler to be used to dispath the Pod | |||
| //+optional | |||
There was a problem hiding this comment.
two minor nits:
dispath -> dispatch
//+optional -> // +optional
There was a problem hiding this comment.
I will make these changes. Thanks for checking thi out.
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: sbwsg The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
tekton-robot
added
the
approved
|
@waveywaves thanks for the PR! Once this is rebased on top of latest master and nits addressed I'll /lgtm and we should be able to get this merged. thanks again |
Set scheduler in PodTemplate to execute Tasks with specific schedulers.
This test checks if the schedulerName has passed down from the TaskRunSpec to the PodSpec
waveywaves
force-pushed
the
feat/scheduler-name
branch
from
c842b3f to
95d9cb4
Compare
|
@vdemeester Can you PTAL at the docs ? Let me know if there need to be any changes. |
waveywaves
force-pushed
the
feat/scheduler-name
branch
from
c7f12a6 to
1b75fd6
Compare
|
@sbwsg I have updated the PR with the necessary changes. Let me know if I need to make more changes. |
|
/lgtm |
|
/test pull-tekton-pipeline-integration-tests |
|
Just curious, has anyone tested this with kube-batch/volcano? |
Changes
Set scheduler in TaskRunSpec to execute Tasks with specific schedulers.
fixes #1739
Submitter Checklist
These are the criteria that every PR should meet, please check them off as you
review them:
Reviewer Notes
If API changes are included, additive changes must be approved by at least two OWNERS and backwards incompatible changes must be approved by more than 50% of the OWNERS, and they must first be added in a backwards compatible way.
Release Notes
SchedulerNameadded to TaskRunSpecSchedulerNameset inTaskRunSpecrecorded byPodso that thePodis dispatched by the mentioned schduler.